From 708918ccbf7ecae70dbf077d7460787d35993f5e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Sun, 26 Apr 2015 00:43:37 +0200 Subject: [PATCH] Provide detailed information about invalid titles includes/Title.php * The private method Title::secureAndSplit() now throws a MalformedTitleException instead of returning false on invalid titles. * Added Title::newFromTextThrow(), which behaves exactly like Title::newFromText() but throws MalformedTitleException instead of returning null on invalid titles. includes/title/MediaWikiTitleCodec.php * Provide more information with the thrown MalformedTitleExceptions. includes/MediaWiki.php * Use the new Title::newFromTextThrow() to get detailed error information, display it. Change-Id: I4da8ecb457a77473e32d745ba48ab8505b35e45f --- includes/MediaWiki.php | 22 ++++++- includes/Title.php | 71 +++++++++++++------- includes/exception/BadTitleError.php | 20 +++++- includes/title/MalformedTitleException.php | 45 +++++++++++-- includes/title/MediaWikiTitleCodec.php | 33 +++++----- languages/i18n/en.json | 9 +++ languages/i18n/qqq.json | 9 +++ tests/phpunit/includes/TitleTest.php | 76 ++++++++++++---------- 8 files changed, 199 insertions(+), 86 deletions(-) diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php index ec2f40f626..4c44121f74 100644 --- a/includes/MediaWiki.php +++ b/includes/MediaWiki.php @@ -51,6 +51,7 @@ class MediaWiki { /** * Parse the request to get the Title object * + * @throws MalformedTitleException If a title has been provided by the user, but is invalid. * @return Title Title object to be $wgTitle */ private function parseTitle() { @@ -110,7 +111,10 @@ class MediaWiki { } if ( $ret === null || ( $ret->getDBkey() == '' && !$ret->isExternal() ) ) { - $ret = SpecialPage::getTitleFor( 'Badtitle' ); + // If we get here, we definitely don't have a valid title; throw an exception. + // Try to get detailed invalid title exception first, fall back to MalformedTitleException. + Title::newFromTextThrow( $title ); + throw new MalformedTitleException( null, $title ); } return $ret; @@ -122,7 +126,11 @@ class MediaWiki { */ public function getTitle() { if ( !$this->context->hasTitle() ) { - $this->context->setTitle( $this->parseTitle() ); + try { + $this->context->setTitle( $this->parseTitle() ); + } catch ( MalformedTitleException $ex ) { + $this->context->setTitle( SpecialPage::getTitleFor( 'Badtitle' ) ); + } } return $this->context->getTitle(); } @@ -174,6 +182,11 @@ class MediaWiki { || $title->isSpecial( 'Badtitle' ) ) { $this->context->setTitle( SpecialPage::getTitleFor( 'Badtitle' ) ); + try { + $this->parseTitle(); + } catch ( MalformedTitleException $ex ) { + throw new BadTitleError( $ex ); + } throw new BadTitleError(); } @@ -219,6 +232,11 @@ class MediaWiki { $output->redirect( $url, 301 ); } else { $this->context->setTitle( SpecialPage::getTitleFor( 'Badtitle' ) ); + try { + $this->parseTitle(); + } catch ( MalformedTitleException $ex ) { + throw new BadTitleError( $ex ); + } throw new BadTitleError(); } // Redirect loops, no title in URL, $wgUsePathInfo URLs, and URLs with a variant diff --git a/includes/Title.php b/includes/Title.php index b0df15f044..26a1b4f96d 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -225,9 +225,11 @@ class Title { public static function newFromDBkey( $key ) { $t = new Title(); $t->mDbkeyform = $key; - if ( $t->secureAndSplit() ) { + + try { + $t->secureAndSplit(); return $t; - } else { + } catch ( MalformedTitleException $ex ) { return null; } } @@ -266,6 +268,32 @@ class Title { wfWarn( __METHOD__ . ': $text must be a string. This will throw an InvalidArgumentException in future.', 2 ); } + try { + return Title::newFromTextThrow( $text, $defaultNamespace ); + } catch ( MalformedTitleException $ex ) { + return null; + } + } + + /** + * Like Title::newFromText(), but throws MalformedTitleException when the title is invalid, + * rather than returning null. + * + * The exception subclasses encode detailed information about why the title is invalid. + * + * @see Title::newFromText + * + * @since 1.25 + * @param string $text Title text to check + * @param int $defaultNamespace + * @throws MalformedTitleException If the title is invalid + * @return Title + */ + public static function newFromTextThrow( $text, $defaultNamespace = NS_MAIN ) { + if ( is_object( $text ) ) { + throw new MWException( 'Title::newFromTextThrow given an object' ); + } + $cache = self::getTitleCache(); /** @@ -287,14 +315,11 @@ class Title { $t->mDbkeyform = str_replace( ' ', '_', $filteredText ); $t->mDefaultNamespace = intval( $defaultNamespace ); - if ( $t->secureAndSplit() ) { - if ( $defaultNamespace == NS_MAIN ) { - $cache->set( $text, $t ); - } - return $t; - } else { - return null; + $t->secureAndSplit(); + if ( $defaultNamespace == NS_MAIN ) { + $cache->set( $text, $t ); } + return $t; } /** @@ -323,9 +348,11 @@ class Title { } $t->mDbkeyform = str_replace( ' ', '_', $url ); - if ( $t->secureAndSplit() ) { + + try { + $t->secureAndSplit(); return $t; - } else { + } catch ( MalformedTitleException $ex ) { return null; } } @@ -507,9 +534,11 @@ class Title { $t = new Title(); $t->mDbkeyform = Title::makeName( $ns, $title, $fragment, $interwiki, true ); - if ( $t->secureAndSplit() ) { + + try { + $t->secureAndSplit(); return $t; - } else { + } catch ( MalformedTitleException $ex ) { return null; } } @@ -3310,6 +3339,7 @@ class Title { * namespace prefixes, sets the other forms, and canonicalizes * everything. * + * @throws MalformedTitleException On invalid titles * @return bool True on success */ private function secureAndSplit() { @@ -3320,15 +3350,12 @@ class Title { $dbkey = $this->mDbkeyform; - try { - // @note: splitTitleString() is a temporary hack to allow MediaWikiTitleCodec to share - // the parsing code with Title, while avoiding massive refactoring. - // @todo: get rid of secureAndSplit, refactor parsing code. - $titleParser = self::getTitleParser(); - $parts = $titleParser->splitTitleString( $dbkey, $this->getDefaultNamespace() ); - } catch ( MalformedTitleException $ex ) { - return false; - } + // @note: splitTitleString() is a temporary hack to allow MediaWikiTitleCodec to share + // the parsing code with Title, while avoiding massive refactoring. + // @todo: get rid of secureAndSplit, refactor parsing code. + $titleParser = self::getTitleParser(); + // MalformedTitleException can be thrown here + $parts = $titleParser->splitTitleString( $dbkey, $this->getDefaultNamespace() ); # Fill fields $this->setFragment( '#' . $parts['fragment'] ); diff --git a/includes/exception/BadTitleError.php b/includes/exception/BadTitleError.php index e62f8bd6fa..4710022d9f 100644 --- a/includes/exception/BadTitleError.php +++ b/includes/exception/BadTitleError.php @@ -28,11 +28,26 @@ */ class BadTitleError extends ErrorPageError { /** - * @param string|Message $msg A message key (default: 'badtitletext') + * @param string|Message|MalformedTitleException $msg A message key (default: 'badtitletext'), or + * a MalformedTitleException to figure out things from * @param array $params Parameter to wfMessage() */ public function __construct( $msg = 'badtitletext', $params = array() ) { - parent::__construct( 'badtitle', $msg, $params ); + if ( $msg instanceof MalformedTitleException ) { + $errorMessage = $msg->getErrorMessage(); + if ( !$errorMessage ) { + parent::__construct( 'badtitle', 'badtitletext', array() ); + } else { + $errorMessageParams = $msg->getErrorMessageParameters(); + $titleText = $msg->getTitleText(); + if ( $titleText ) { + $errorMessageParams[] = $titleText; + } + parent::__construct( 'badtitle', $errorMessage, $errorMessageParams ); + } + } else { + parent::__construct( 'badtitle', $msg, $params ); + } } /** @@ -47,5 +62,4 @@ class BadTitleError extends ErrorPageError { $wgOut->setStatusCode( 400 ); parent::report(); } - } diff --git a/includes/title/MalformedTitleException.php b/includes/title/MalformedTitleException.php index a9e58b3e56..e7477781b7 100644 --- a/includes/title/MalformedTitleException.php +++ b/includes/title/MalformedTitleException.php @@ -1,7 +1,5 @@ errorMessage = $errorMessage; + $this->titleText = $titleText; + $this->errorMessageParameters = $errorMessageParameters; + } + + /** + * @since 1.26 + * @return string|null + */ + public function getTitleText() { + return $this->titleText; + } + + /** + * @since 1.26 + * @return string|null + */ + public function getErrorMessage() { + return $this->errorMessage; + } + + /** + * @since 1.26 + * @return string[] + */ + public function getErrorMessageParameters() { + return $this->errorMessageParameters; + } } diff --git a/includes/title/MediaWikiTitleCodec.php b/includes/title/MediaWikiTitleCodec.php index 20034b7492..98cec59633 100644 --- a/includes/title/MediaWikiTitleCodec.php +++ b/includes/title/MediaWikiTitleCodec.php @@ -137,12 +137,12 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { // Interwiki links are not supported by TitleValue if ( $parts['interwiki'] !== '' ) { - throw new MalformedTitleException( 'Title must not contain an interwiki prefix: ' . $text ); + throw new MalformedTitleException( 'title-invalid-interwiki', $text ); } // Relative fragment links are not supported by TitleValue if ( $parts['dbkey'] === '' ) { - throw new MalformedTitleException( 'Title must not be empty: ' . $text ); + throw new MalformedTitleException( 'title-invalid-empty', $text ); } return new TitleValue( $parts['namespace'], $parts['dbkey'], $parts['fragment'] ); @@ -232,7 +232,7 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { if ( strpos( $dbkey, UtfNormal\Constants::UTF8_REPLACEMENT ) !== false ) { # Contained illegal UTF-8 sequences or forbidden Unicode chars. - throw new MalformedTitleException( 'Bad UTF-8 sequences found in title: ' . $text ); + throw new MalformedTitleException( 'title-invalid-utf8', $text ); } $parts['dbkey'] = $dbkey; @@ -246,7 +246,7 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { } if ( $dbkey == '' ) { - throw new MalformedTitleException( 'Empty title: ' . $text ); + throw new MalformedTitleException( 'title-invalid-empty', $text ); } # Namespace or interwiki prefix @@ -263,11 +263,11 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { if ( $ns == NS_TALK && preg_match( $prefixRegexp, $dbkey, $x ) ) { if ( $this->language->getNsIndex( $x[1] ) ) { # Disallow Talk:File:x type titles... - throw new MalformedTitleException( 'Bad namespace prefix: ' . $text ); + throw new MalformedTitleException( 'title-invalid-talk-namespace', $text ); } elseif ( Interwiki::isValidInterwiki( $x[1] ) ) { //TODO: get rid of global state! # Disallow Talk:Interwiki:x type titles... - throw new MalformedTitleException( 'Interwiki prefix found in title: ' . $text ); + throw new MalformedTitleException( 'title-invalid-talk-namespace', $text ); } } } elseif ( Interwiki::isValidInterwiki( $p ) ) { @@ -324,8 +324,9 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { # Reject illegal characters. $rxTc = self::getTitleInvalidRegex(); - if ( preg_match( $rxTc, $dbkey ) ) { - throw new MalformedTitleException( 'Illegal characters found in title: ' . $text ); + $matches = array(); + if ( preg_match( $rxTc, $dbkey, $matches ) ) { + throw new MalformedTitleException( 'title-invalid-characters', $text, array( $matches[0] ) ); } # Pages with "/./" or "/../" appearing in the URLs will often be un- @@ -343,23 +344,21 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { substr( $dbkey, -3 ) == '/..' ) ) { - throw new MalformedTitleException( 'Bad title: ' . $text ); + throw new MalformedTitleException( 'title-invalid-relative', $text ); } # Magic tilde sequences? Nu-uh! if ( strpos( $dbkey, '~~~' ) !== false ) { - throw new MalformedTitleException( 'Bad title: ' . $text ); + throw new MalformedTitleException( 'title-invalid-magic-tilde', $text ); } # Limit the size of titles to 255 bytes. This is typically the size of the # underlying database field. We make an exception for special pages, which # don't need to be stored in the database, and may edge over 255 bytes due # to subpage syntax for long titles, e.g. [[Special:Block/Long name]] - if ( - ( $parts['namespace'] != NS_SPECIAL && strlen( $dbkey ) > 255 ) - || strlen( $dbkey ) > 512 - ) { - throw new MalformedTitleException( 'Title too long: ' . substr( $dbkey, 0, 255 ) . '...' ); + $maxLength = ( $parts['namespace'] != NS_SPECIAL ) ? 255 : 512; + if ( strlen( $dbkey ) > $maxLength ) { + throw new MalformedTitleException( 'title-invalid-too-long', $text, array( $maxLength ) ); } # Normally, all wiki links are forced to have an initial capital letter so [[foo]] @@ -374,7 +373,7 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { # self-links with a fragment identifier. if ( $dbkey == '' && $parts['interwiki'] === '' ) { if ( $parts['namespace'] != NS_MAIN ) { - throw new MalformedTitleException( 'Empty title: ' . $text ); + throw new MalformedTitleException( 'title-invalid-empty', $text ); } } @@ -390,7 +389,7 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser { // Any remaining initial :s are illegal. if ( $dbkey !== '' && ':' == $dbkey[0] ) { - throw new MalformedTitleException( 'Title must not start with a colon: ' . $text ); + throw new MalformedTitleException( 'title-invalid-leading-colon', $text ); } # Fill fields diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 8afed1a44c..d8554b251e 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -340,6 +340,15 @@ "no-null-revision": "Could not create new null revision for page \"$1\"", "badtitle": "Bad title", "badtitletext": "The requested page title was invalid, empty, or an incorrectly linked inter-language or inter-wiki title.\nIt may contain one or more characters that cannot be used in titles.", + "title-invalid-empty": "The requested page title is empty or contains only the name of a namespace.", + "title-invalid-utf8": "The requested page title contains an invalid UTF-8 sequence.", + "title-invalid-interwiki": "Title contains an interwiki link", + "title-invalid-talk-namespace": "The requested page title refers to a talk page that can not exist.", + "title-invalid-characters": "The requested page title contains invalid characters: \"$1\".", + "title-invalid-relative": "Title has relative path. Relative page titles (./, ../) are invalid, because they will often be unreachable when handled by user\"s browser.", + "title-invalid-magic-tilde": "The requested page title contains invalid magic tilde sequence (~~~).", + "title-invalid-too-long": "The requested page title is too long. It must be no longer than $1 bytes in UTF-8 encoding.", + "title-invalid-leading-colon": "The requested page title contains an invalid colon at the beginning.", "perfcached": "The following data is cached and may not be up to date. A maximum of {{PLURAL:$1|one result is|$1 results are}} available in the cache.", "perfcachedts": "The following data is cached, and was last updated $1. A maximum of {{PLURAL:$4|one result is|$4 results are}} available in the cache.", "querypage-no-updates": "Updates for this page are currently disabled.\nData here will not presently be refreshed.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index a7b55243a1..da9a8ac2e5 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -507,6 +507,15 @@ "delete-hook-aborted": "Error message shown when an extension hook prevents a page deletion, but does not provide an error message.", "no-null-revision": "Error message shown when no null revision could be created to reflect a protection level change.\n\nAbout \"null revision\":\n* Create a new null-revision for insertion into a page's history. This will not re-save the text, but simply refer to the text from the previous version.\n* Such revisions can for instance identify page rename operations and other such meta-modifications.\n\nParameters:\n* $1 - page title", "badtitle": "The page title when a user requested a page with invalid page name. The content will be {{msg-mw|badtitletext}}.", + "title-invalid-empty": "Used as text of error message: empty title", + "title-invalid-utf8": "Used as text of error message: invalid UTF8 sequence", + "title-invalid-interwiki": "Used as text of error message: invalid interwiki link", + "title-invalid-talk-namespace": "Used as text of error message: invalid talk page", + "title-invalid-characters": "Used as text of error message: invalid characters in title ($1 is the character)", + "title-invalid-relative": "Used as text of error message: relative titles are invalid", + "title-invalid-magic-tilde": "Used as text of error message: magic tilde sequence is invalid in page title", + "title-invalid-too-long": "Used as text of error message: too long title ($1 is maximum length)", + "title-invalid-leading-colon": "Used as text of error message: colon at the beginning of title is invalid", "badtitletext": "The message shown when a user requested a page with invalid page name. The page title will be {{msg-mw|badtitle}}.\n\nSee also:\n* {{msg-mw|selfmove}}\n* {{msg-mw|immobile-source-namespace}}\n* {{msg-mw|immobile-target-namespace-iw}}\n* {{msg-mw|immobile-target-namespace}}", "perfcached": "Like {{msg-mw|perfcachedts}} but used when we do not know how long ago page was cached (unlikely to happen).\n\nParameters:\n* $1 - the max result cut off ($wgQueryCacheLimit)", "perfcachedts": "Used on pages that list page lists for which the displayed data is cached. Parameters:\n* $1 - a time stamp (date and time combined)\n* $2 - a date (optional)\n* $3 - a time (optional)\n* $4 - the cut off limit for cached results ($wgQueryCacheLimit). If there are more then this many results for the query, only the first $4 of those will be listed on the page. Usually $4 is about 1000.", diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php index 00c29ee42a..74a741ad40 100644 --- a/tests/phpunit/includes/TitleTest.php +++ b/tests/phpunit/includes/TitleTest.php @@ -80,22 +80,22 @@ class TitleTest extends MediaWikiTestCase { public static function provideInvalidSecureAndSplit() { return array( - array( '' ), - array( ':' ), - array( '__ __' ), - array( ' __ ' ), + array( '', 'title-invalid-empty' ), + array( ':', 'title-invalid-empty' ), + array( '__ __', 'title-invalid-empty' ), + array( ' __ ', 'title-invalid-empty' ), // Bad characters forbidden regardless of wgLegalTitleChars - array( 'A [ B' ), - array( 'A ] B' ), - array( 'A { B' ), - array( 'A } B' ), - array( 'A < B' ), - array( 'A > B' ), - array( 'A | B' ), + array( 'A [ B', 'title-invalid-characters' ), + array( 'A ] B', 'title-invalid-characters' ), + array( 'A { B', 'title-invalid-characters' ), + array( 'A } B', 'title-invalid-characters' ), + array( 'A < B', 'title-invalid-characters' ), + array( 'A > B', 'title-invalid-characters' ), + array( 'A | B', 'title-invalid-characters' ), // URL encoding - array( 'A%20B' ), - array( 'A%23B' ), - array( 'A%2523B' ), + array( 'A%20B', 'title-invalid-characters' ), + array( 'A%23B', 'title-invalid-characters' ), + array( 'A%2523B', 'title-invalid-characters' ), // XML/HTML character entity references // Note: Commented out because they are not marked invalid by the PHP test as // Title::newFromText runs Sanitizer::decodeCharReferencesAndNormalize first. @@ -103,29 +103,30 @@ class TitleTest extends MediaWikiTestCase { //'A é B', //'A é B', // Subject of NS_TALK does not roundtrip to NS_MAIN - array( 'Talk:File:Example.svg' ), + array( 'Talk:File:Example.svg', 'title-invalid-talk-namespace' ), // Directory navigation - array( '.' ), - array( '..' ), - array( './Sandbox' ), - array( '../Sandbox' ), - array( 'Foo/./Sandbox' ), - array( 'Foo/../Sandbox' ), - array( 'Sandbox/.' ), - array( 'Sandbox/..' ), + array( '.', 'title-invalid-relative' ), + array( '..', 'title-invalid-relative' ), + array( './Sandbox', 'title-invalid-relative' ), + array( '../Sandbox', 'title-invalid-relative' ), + array( 'Foo/./Sandbox', 'title-invalid-relative' ), + array( 'Foo/../Sandbox', 'title-invalid-relative' ), + array( 'Sandbox/.', 'title-invalid-relative' ), + array( 'Sandbox/..', 'title-invalid-relative' ), // Tilde - array( 'A ~~~ Name' ), - array( 'A ~~~~ Signature' ), - array( 'A ~~~~~ Timestamp' ), - array( str_repeat( 'x', 256 ) ), + array( 'A ~~~ Name', 'title-invalid-magic-tilde' ), + array( 'A ~~~~ Signature', 'title-invalid-magic-tilde' ), + array( 'A ~~~~~ Timestamp', 'title-invalid-magic-tilde' ), + // Length + array( str_repeat( 'x', 256 ), 'title-invalid-too-long' ), // Namespace prefix without actual title - array( 'Talk:' ), - array( 'Talk:#' ), - array( 'Category: ' ), - array( 'Category: #bar' ), + array( 'Talk:', 'title-invalid-empty' ), + array( 'Talk:#', 'title-invalid-empty' ), + array( 'Category: ', 'title-invalid-empty' ), + array( 'Category: #bar', 'title-invalid-empty' ), // interwiki prefix - array( 'localtestiw: Talk: # anchor' ), - array( 'localtestiw: Talk:' ) + array( 'localtestiw: Talk: # anchor', 'title-invalid-empty' ), + array( 'localtestiw: Talk:', 'title-invalid-empty' ) ); } @@ -164,9 +165,14 @@ class TitleTest extends MediaWikiTestCase { * @dataProvider provideInvalidSecureAndSplit * @note This mainly tests MediaWikiTitleCodec::parseTitle(). */ - public function testSecureAndSplitInvalid( $text ) { + public function testSecureAndSplitInvalid( $text, $expectedErrorMessage ) { $this->secureAndSplitGlobals(); - $this->assertNull( Title::newFromText( $text ), "Invalid: $text" ); + try { + Title::newFromTextThrow( $text ); // should throw + $this->assertTrue( false, "Invalid: $text" ); + } catch ( MalformedTitleException $ex ) { + $this->assertEquals( $expectedErrorMessage, $ex->getErrorMessage(), "Invalid: $text" ); + } } public static function provideConvertByteClassToUnicodeClass() { -- 2.20.1